-
Notifications
You must be signed in to change notification settings - Fork 107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rename Amazon block storage manager #107
Conversation
Initial implementation of the block storage manager presumed that Amazon provider would only have a single block storage manager and consequently used the generic class name `Amazon::BlockStorageManager`. @rolivery suggested to change the name to indicate the actual type of the manager, which in this case is EBS. This patch thus replaces all occurrences where `BlockStorageManager` was used before with `StorageManager::Ebs`. Signed-off-by: Gregor Berginc <[email protected]>
Since we are now explicitly adding EBS provider, this patch changes the name of the corresponding dependent block storage manager. Signed-off-by: Gregor Berginc <[email protected]>
067f20e
to
b8f8ba1
Compare
@roliveri Although you proposed If there is a solution to this, I can revert to EBS. Related PRs in the manageiq repository:
|
Following the renaming of the EBS storage manager, this patch also changes the EBS storage key in the `config/settings.yml` file. Signed-off-by: Gregor Berginc <[email protected]>
Checked commits gberginc/manageiq-providers-amazon@99efe4b~...7cf0783 with ruby 2.2.6, rubocop 0.46.0, and haml-lint 0.19.0 app/models/manageiq/providers/amazon/inventory/targets/storage_manager/ebs.rb
app/models/manageiq/providers/amazon/storage_manager/ebs/refresh_parser.rb
app/models/manageiq/providers/amazon/storage_manager/ebs/refresh_parser_inventory_object.rb
app/models/manageiq/providers/amazon/storage_manager/ebs/refresher.rb
spec/models/manageiq/providers/amazon/storage_manager/ebs/stubbed_refresher_spec.rb
|
@gberginc @Ladas This looks good to me. |
@roliveri I have removed WIP status of the other PRs on manageiq repo (listed below). Before merging this one, I would like to get LGTMs on the other PRs. I think the order of merges should be:
|
@gberginc nice rename :-) |
@roliveri merged, you can proceed :-) |
Based on this comment from @roliveri, this PR replaces the current
BlockStorageManager
withStorageManager::EBS
. This has two benefits. First, it explicitly names the block storage type, which is Elastic Block Store, and second, it allows for different types of block storage managers to be used by the amazon provider.Changes were made in the provider itself as well as the inventory targets.
I am also suggesting changes in the cloud manager, i.e. replacing
block_storage_manager
withebs_storage_manager
. Please check whether these changes also make sense.@Ladas @roliveri: please review. As soon as we agree on the structure, I will provide changes for the manageiq repository (automate and ext management system).
Before merging, we should sync to prevent failures in the main repo that occurred with #101 today (for which I apologise).
@miq-bot add_label euwe/no